Skip to content

fix: Save lock-token from receive_deferred_messages AMQP payload, into ServiceBusReceivedMessage #42471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: Save lock-token from receive_deferred_messages AMQP payload, into ServiceBusReceivedMessage #42471

wants to merge 2 commits into from

Conversation

@ashleysommer
Copy link

@ashleysommer ashleysommer commented Aug 12, 2025

Description

Save lock-token from receive_deferred_messages AMQP payload, so it can be used as the message's lock_token property. This allows the deferred message to be correctly renewed or settled (abandoned, completed, deferred again).

Fixes #42454

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

This is for initial review. I have not included anything in Changelog.
There are no tests at all in the codebase for deferred messages or anything to do with parsing deferred messages, so I don't know where to add a regression test for this.

@ashleysommer
Save lock-token from receive_deferred_message AMQP payload, so it c…
ffc6d2f 
…an be used as the message's lock_token property. This allows the deferred message to be correctly renewed or settled (abandoned, completed, deferred again).
Copilot AI review requested due to automatic review settings August 12, 2025 02:45
@github-actions github-actions bot added Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Aug 12, 2025
@github-actions
Copy link

github-actions bot commented Aug 12, 2025

Thank you for your contribution @ashleysommer! We will review the pull request and get back to you soon.

This comment was marked as outdated.

Sign in to view

@ashleysommer
Copy link
Author

ashleysommer commented Aug 12, 2025

@microsoft-github-policy-service agree

@ashleysommer
Use kwargs.get() instead of kwargs.pop() to get the optional rece…
48cc1cd 
…ived lock token in the constructor

Co-authored-by: Copilot <[email protected]>
@ashleysommer ashleysommer mentioned this pull request Aug 12, 2025
Open
@ashleysommer ashleysommer requested a review from Copilot August 12, 2025 03:18
Copilot AI reviewed Aug 12, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug where deferred Service Bus messages were not properly retaining their lock tokens from the AMQP payload, preventing proper message renewal and settlement operations. The fix ensures that when receiving deferred messages, the lock token from the AMQP response is correctly extracted and assigned to the ServiceBusReceivedMessage object.

Key changes:

  • Extract lock token from AMQP payload for deferred messages in both transport implementations
  • Store the received lock token in the message object during initialization
  • Update the lock_token property to prioritize the received lock token over the delivery tag

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
_uamqp_transport.py Extract lock token from deferred message AMQP payload and pass to message constructor
_pyamqp_transport.py Extract lock token from deferred message AMQP payload and pass to message constructor
message.py Store received lock token and prioritize it in the lock_token property

@github-actions
Copy link

github-actions bot commented Oct 17, 2025

Hi @ashleysommer. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

@github-actions github-actions bot added the no-recent-activity There has been no recent activity on this issue. label Oct 17, 2025
@ashleysommer
Copy link
Author

ashleysommer commented Oct 17, 2025

Yes, this PR is still needed, still waiting for a human to review it.

@github-actions github-actions bot removed the no-recent-activity There has been no recent activity on this issue. label Oct 17, 2025
@ashleysommer
Copy link
Author

ashleysommer commented Nov 5, 2025

This still needs human review, and needs someone to complete the final parts of adding regression tests and writing changelog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@annatisch annatisch Awaiting requested review from annatisch

@kashifkhan kashifkhan Awaiting requested review from kashifkhan

@swathipil swathipil Awaiting requested review from swathipil

@l0lawrence l0lawrence Awaiting requested review from l0lawrence

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. Service Bus

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ServiceBus] Message received from receive_deferred_messages does not have a lock token.

1 participant

@ashleysommer